Skip to content

Syncing from upstream saltstack/salt (feature/nightly-stress-3008x-dispatch) - #143

Open
bt-admin wants to merge 535 commits into
brain-tec:feature/nightly-stress-3008x-dispatchfrom
saltstack:feature/nightly-stress-3008x-dispatch
Open

Syncing from upstream saltstack/salt (feature/nightly-stress-3008x-dispatch)#143
bt-admin wants to merge 535 commits into
brain-tec:feature/nightly-stress-3008x-dispatchfrom
saltstack:feature/nightly-stress-3008x-dispatch

Conversation

@bt-admin

Copy link
Copy Markdown

bt_gitbot

dwoz and others added 30 commits August 12, 2026 16:40
The pkg-test downgrade matrix installs current salt, then rolls back
to the previous release before running pytest. That leaves the
pre-#69950 onedir on disk, which correctly lacks libyaml-linked
PyYAML — asserting its presence there produces a false positive.

Gate both libyaml tests on install_salt.downgrade so the downgrade
matrix skips them; install/upgrade flavors continue to exercise the
fix.
…owngrade

Post-downgrade pytest re-enters the pkg/integration suite with the
previous salt onedir on disk and no --downgrade flag, so the earlier
install_salt.downgrade guard didn't trigger and the test still failed.

Rewritten to key on install_salt.version:
- >= 3006.28 -> assert libyaml present (guards the fix)
- <  3006.28 -> assert libyaml absent (documents the pre-fix baseline
  so a silent regression on the old branch is also caught)

Works uniformly across the three pkg-test flavors without skips.
The prior design compared install_salt.version >= Version("3006.28") to
predict whether libyaml should be present, but dev builds report
'3006.27+NNN.gSHA' which packaging.version orders BEFORE '3006.27' let
alone '3006.28' (PEP 440 local-version segment). That flipped the
expected-libyaml boolean to False on install jobs and turned every
Linux install matrix row red.

install_salt.use_prev_version is True iff the pytest run is executing
against the downgraded-to previous release (set by --use-prev-version in
the post-downgrade validation stage). That's the only flavor where the
onedir predates PR #69950 and libyaml is legitimately absent. Key the
expectation off that flag and drop the version-comparison plumbing.
…69920)

Give each daemon AsyncReqMessageClient a per-instance uuid.uuid4().hex
as its ZMQ IDENTITY, replacing the earlier process-wide
_REQ_IDENTITY_SLOT counter and SALT_REQ_IDENTITY_SLOT_MAX cap.  Each
RequestClient is opened and closed by Salt itself, so a per-instance
UUID matches the object's lifetime and gives the master ROUTER's
routing-id table a 1:1 mapping to a client we control.  Fork
inheritance of the earlier counter -- root cause of #69753 -- is
impossible by construction, since each child draws a fresh UUID.
path= stripped PEM headers while key= hashed the raw string, so
master_finger from a key string did not match salt-key -F.
Fixes #69970
`salt-pip` shells out to `python -m pip` against a packager-pinned onedir
pip; pip's periodic "A new release of pip is available" HTTPS check is
pure noise (the user can't do anything about it) and a proxy-config
gotcha (cf. #69910). `salt/modules/pip.py` already suppresses it on 6
install/list/upgrade paths; salt-pip should too.

Set `PIP_DISABLE_PIP_VERSION_CHECK=1` in `_pip_environment` via
`setdefault` so operators can opt back in by exporting
`PIP_DISABLE_PIP_VERSION_CHECK=0` before invoking salt-pip.

Fixes #70024
The unit tests on `_pip_environment` prove the helper injects
`PIP_DISABLE_PIP_VERSION_CHECK=1` (and respects an operator override)
but would still pass if a future refactor stopped routing `salt_pip`
through `_pip_environment`. Add two end-to-end tests that drive
`salt.scripts.salt_pip` with a stubbed `subprocess.run` and assert on
the `env` dict actually handed to the child `python -m pip` process.

Refs #70024
Adds opt-in minion_memory_headroom (accepts "5%" / "5G" / int bytes) and
minion_memory_max (bytes / size string) config options with silent
cgroup v1 / v2 detection. When either opt is set, the reference "total
memory available" is resolved from config > cgroup-v2 > cgroup-v1 >
psutil.virtual_memory().total, and used bytes come from the matching
tier. When neither opt is set the check preserves the legacy
psutil.virtual_memory().percent > 95 behavior byte-for-byte, so no
minion changes behavior on upgrade.

Refs #69884
resource_modules() built its LazyLoader over _module_dirs(), which returns
the full stock module set (~100+ salt/modules/*) with per-type overlay
directories merely prepended.  That left every stock function reachable
in a resource-scoped loader.  Targeting a resource with a name like
cmd.run / grains.setval / file.remove / state.apply resolved through the
resource loader, ran the stock function in the managing minion process,
and returned the result attributed to the resource id — contradicting
the documented Resources safety contract (unsupported functions must
fail loudly; managing-minion access is explicit via __minion__).

Add a new private helper _resource_type_module_dirs() that walks the
same layer stack _module_dirs() walks (cli module_dirs, extension_modules,
entry-point packages, SALT_BASE_PATH) but only accepts each layer's
resources/<rtype>/<ext_type>/ overlay subdir — never the layer's plain
<ext_type>/ dir.  resource_modules() now uses this helper.

Result:

* salt <resource-id> <stock-fun> — "Function 'X' is not supported for
  resource type 'Y'." (the _thread_return guard fires because the name
  is absent from the loader).
* salt <resource-id> <per-type-fun> — reachable as before via the
  resources/<rtype>/modules/ overlay.
* Resource-context modules that intentionally want managing-minion
  behavior still call __minion__["x.y"] explicitly.

Tests:

* tests/pytests/unit/loader/test_per_resource_overrides.py
  - Flip test_no_override_falls_through_to_standard_state_module (which
    asserted stock state.sls was present) → test_no_override_hides_stock_modules
    asserting no stock cmd/state/grains/file/… leak through.  The prior
    assertion documented the buggy contract.
  - Add test_per_type_override_present_and_callable for the positive path.
* tests/pytests/unit/cli/test_caller_resources.py
  - salt-call -r grains.items / state.apply against a dummy resource
    now returns "not supported for resource type 'dummy'" rejections
    instead of stock function results.
* tests/pytests/integration/resources/test_resource_loader_strict.py (new)
  - End-to-end master+minion coverage: salt dummy-01 cmd.run /
    grains.setval / file.remove / sys.list_functions all return the
    per-type rejection; test.ping (a real per-type override) still runs;
    grains.setval leaves the managing minion's grains untouched.

Fixes #69881
Complements the unit-level matrix in
tests/pytests/unit/test_minion_memory_headroom.py by booting a real
minion daemon with each opt combination and driving the runtime check
via a subprocess that loads the minion's on-disk config through
salt.config.minion_config and calls Minion._has_memory_headroom.

Four scenarios:
  * config round-trip via salt-call --local config.get
  * default preserved (no opts -> legacy True on any healthy host)
  * config override -> deterministic True (huge max, tiny reserve)
  * config override -> deterministic False (tiny max, full reserve)

All scenarios use deterministic config values so no system-memory
pressure is required.

Refs #69884
napalm.junos_cli defaults dev_timeout to None and forwards it, so the Junos
_timeout_decorator / _timeout_decorator_cleankwargs wrappers hit
max(None, 0) -- which raises TypeError -- and would otherwise try to set the
junos-eznc connection timeout to None, which it rejects.

Coalesce None to 0 when computing the effective timeout, and only override the
connection timeout when a real (>0) dev_timeout/timeout is given; otherwise run
the command with the connection's default timeout. This makes junos_cli and the
other timeout-decorated calls work when no timeout is passed, while still
honouring an explicit dev_timeout/timeout.
…65867)

junos.rpc (used by napalm.junos_rpc) builds the RPC ``op`` dict from
__pub_arg, which carries the reserved ``__kwarg__`` marker that the Salt CLI
appends to keyword arguments. On a ``get-config`` call with a ``filter`` the
marker leaked into the RPC options, and junos-eznc's ElementMaker raised
``KeyError: <class 'bool'>`` while rendering the ``True`` value as an XML
attribute, so the filter option stopped working after upgrading from 3004.

Strip dunder keys from ``op`` with salt.utils.args.clean_kwargs (already used
by junos.diff) right after it is assembled, so reserved markers are dropped
on every RPC path. Adds regression tests for the get-config and non
get-config paths.
napalm_ntp (set_peers/set_servers/delete_peers/delete_servers), napalm_snmp
(update_config/remove_config) and napalm_probes (set_probes/delete_probes/
schedule_probes) all call net.load_template with a bare template name (e.g.
"set_ntp_peers"). net.load_template used to route bare names into NAPALM's own
renderer, but that path was removed in the Sodium release (#57370) -- whose own
deprecation warning explicitly told netntp/netsnmp/netusers users to ignore it.
The bare name now falls through to the fileserver as "Local file source
set_ntp_peers does not exist", so every one of these functions fails.

Lift the resolver introduced for users in #62170 into a shared
salt.utils.napalm.template_path (walks the driver class MRO + inspect.getfile,
catching TypeError/OSError, returns None when the driver ships no such template)
plus salt.utils.napalm.template_not_available (standard failure payload). The
three modules resolve the driver's NAPALM-shipped template to an absolute path
and render it through the Salt pipeline, or return the clear failure message.
template_not_available also closes the per-call connection a non-always-alive
proxy/minion opened, since it short-circuits net.load_template (which would
otherwise close it).

napalm_users is intentionally left to #62170; a follow-up dedups its local
resolver onto the shared one.

Validated on a live Juniper EX3400 (Junos 23.4R2): ntp.set_servers renders,
commits, is confirmed in the device config, and is removed again. (NAPALM's
junos snmp/probes templates still use py2 dict.iteritems() and need modernizing
upstream in NAPALM; the NTP templates are py3-clean.)

New tests/pytests/unit/utils/test_napalm.py covers the resolver (MRO
concrete-over-base precedence, base fall-through, OSError/TypeError skip, missing
-> None) and the close behaviour; test_ntp/test_snmp are rewritten and
test_probes added with routing tests asserting the correct template name, the
resolved path, forwarded flags, and inherit_napalm_device identity.
napalm_probes.set_probes/delete_probes/schedule_probes now resolve the driver's
template and no longer return result=True for the bare-name mock, so the legacy
unittest-style tests (which asserted the pre-fix behaviour) fail. They are
superseded by tests/pytests/unit/modules/napalm/test_probes.py added here, which
covers config/results plus the routing and no-template paths.
An always-alive NAPALM proxy runs with multiprocessing disabled, so jobs
executing at the same time are threads that share one cached device object and
its single command channel (get_device returns the same device by reference).
Two concurrent calls -- e.g. net.cli, or a grains refresh landing during a
state run -- can then interleave on that channel, mixing each other's output
and, on drivers that share a raw CLI session without their own locking,
corrupting the connection.

Give each device a reentrant lock, created in get_device(), and hold it in
salt.utils.napalm.call() for the duration of the call. A reentrant lock is
required because call() re-enters itself (close/open/re-exec) on a reconnect;
a plain Lock would deadlock. Devices built without a LOCK (hand-constructed in
tests, or inherited via inherit_napalm_device) run unserialized, unchanged.

The lock is per-device, not global, so a deltaproxy hosting many sub-proxies
in one process does not needlessly serialize calls across unrelated devices.
The EP fan-out hot path did one msgpack.dumps per event (in
frame_msg(package)) followed by a full msgpack.loads on the outer
IPC frame (in TCPPuller) and again on the inner event body (in
SaltEvent.unpack), even though the routing logic only needed to
inspect the event tag.  Under a 50-minion highstate return burst
this produced ~120 MB of transient Python dict/list objects per
burst that stressed the glibc arena, driving EP RSS growth.

Two related changes:

1. raw_payload passthrough (salt/transport/tcp.py):
   TCPPuller.handle_stream reads the length-prefixed msgpack frame
   with raw=True (skips per-key str allocation) and passes the
   original wire bytes as raw_payload=payload to the handler.
   PublishServer.publish_payload and PubServer.publish_payload
   accept and forward raw_payload; when set, PubServer writes
   raw_payload to subscribers instead of re-packing via frame_msg.
   This removes one msgpack.dumps per event on the EP hot path.

2. Tag peek (salt/channel/server.py MasterPubServerChannel.publish_payload):
   Bytes-level load.partition(TAGEND) grabs the tag without
   deserializing the body.  Full salt.payload.loads is called
   lazily via a _decode_data() closure, only in the five
   cluster/runner/* branches that actually need the decoded dict.
   For non-cluster masters (the >99% case), the full unpack is
   never performed.

The cluster-peer fanout path (self.pushers non-empty) still
decodes data via _decode_data() where it needs to wrap the event
in a cluster/event envelope, so cluster deployments retain the
same behavior.  Local fanout forwards raw_payload to
transport.publish_payload; cluster branches do not, because they
mutate load before publishing.

Measured under a 50-minion state.apply/highstate stress rig:
- Peak Python allocation (memray): -28% (124 -> 89 MB)
- Leaked Python bytes (memray): -34% (122 -> 81 MB)
- msgpack.unpackb calls: 6084 -> 66
- Return throughput ceiling: +55% with passthrough alone,
  +111% with tag peek layered on top

Non-cluster deployments see the full benefit.  Cluster
deployments retain identical semantics; the cluster branches
still call _decode_data() before publishing.
…ek (#70052)

Unit + functional coverage for PR #70052:

- salt/transport/tcp.py
  * PubServer.publish_payload writes raw_payload bytes verbatim when
    supplied and skips frame_msg; falls back to frame_msg otherwise.
  * PubServer.publish_payload raw bypass applies with topic_list too.
  * PublishServer.publish_payload forwards raw_payload= kwarg down to
    self.pub_server.publish_payload (default None preserves framing).
  * TCPPuller.handle_stream passes raw_payload=<wire bytes> to the
    handler, falls back to positional-only handler on TypeError, and
    unpacks the outer frame with raw=True (dict keys are bytes).

- salt/channel/server.py MasterPubServerChannel.publish_payload
  * Non-cluster tags (salt/job/..., salt/auth) never invoke
    salt.payload.loads -- verified by patching and asserting .called.
  * raw_payload is forwarded to self.transport.publish_payload for the
    non-cluster local-fanout branch (and defaults to None).
  * All five cluster/runner/* branches (sync_roots, collect_from_peers,
    shed_unowned_all, delegate_write, ring_create/destroy/route_set/
    route_clear/ring_set) invoke _decode_data() and dispatch the
    decoded body into the appropriate _run_* / _handle_multi_ring_*
    method.
  * cluster/peer* tags do not re-broadcast locally.
  * cluster-peer fanout branch (self.pushers non-empty, tag NOT
    cluster/peer*) decodes the body to build the cluster/event
    envelope AND still forwards raw_payload to the local transport.

- tests/pytests/functional/transport/tcp/test_pub_server.py
  * End-to-end regression: real PublishServer + PublishClient +
    TCPPuller path, verifying raw_payload flows from the pull socket
    to the subscriber and the received payload still decodes back to
    the original dict.

Fixes two pre-existing tests that broke on the raw=True unpack switch
(test_tcp_puller_handle_stream_awaits_payload_handler and
test_tcp_puller_handle_stream_survives_handler_exception): body values
now arrive as bytes.
- get_device_opts: optional_args explicitly set to null yielded None (the get
  default only applies to a missing key), crashing the "config_lock" membership
  test; and a present optional_args dict was mutated in place, leaking the
  config_lock / keepalive defaults into the caller's opts/pillar. Use
  copy.deepcopy(device_dict.get("optional_args") or {}).

- proxy_napalm_wrap: force_reconnect did opts["proxy"].update(**kwargs)
  unconditionally, raising KeyError on a straight (non-proxy) minion which has
  no 'proxy' key. That merge is only for the always-alive proxy path; a straight
  minion picks the override up from clean_kwargs, so guard it with is_proxy().

- proxy.shutdown: a trailing comma made 'port' a 1-tuple, so a failed close()
  logged ':(830,)' / ':(None,)'. Remove it.
The NSIS stress tests still hit intermittent installer Abort (exit
code 2) after the previous retry fix, on both 3006.x and 3007.x.
Timing recovered from the CI logs shows the SCM held the
salt-minion service key for 25s+ before the uninstall side's own
wait_svc_deleted loop and the test harness's post-uninstall wait
both gave up, leaving only ~10s of retry budget on the install side
before it aborted -- not enough headroom for the observed delay.
Wait for the salt-minion service registry key to disappear before
even attempting "ssm install" (CreateService), instead of only
reacting after CreateService fails. This is a no-op on a normal
install, since the key was never present. Also widen the existing
retry budgets: the install-side CreateService retry from 5x2s to
10x2s, and the uninstall-side wait_svc_deleted from 10s to 15s.

Add diagnostics so future occurrences don't require a fresh
repro: print the tail of the relevant %TEMP%\SaltInstaller\*.log
directly into the pytest failure output on any non-zero exit or
timeout, and upload the full log directory as a CI artifact from
both the Logic Tests and Stress Tests jobs.
dwoz and others added 29 commits August 27, 2026 14:38
[3008.x] nightly: sign DEB packages with debsigs, mirroring RPM signing path
…name

#70162 marked ARGON released=True on 3008.x so
SaltVersionsInfo.current_release() now returns ARGON. The regression
test test_current_release_matches_maintenance_branch_67061 was
asserting the pre-flip codename by name (CHLORINE), which was
correct for 3007.x but wrong once 3008.x updated the released
flag. Every 3008.x PR started failing here (e.g. #70157 CI).

Rather than bump the hardcoded codename with every release cycle
(and reintroduce this same failure mode next time POTASSIUM or its
successors flip to released=True), assert the *contract* that
current_release() is documented to satisfy: return the last codename
in SaltVersionsInfo.versions() with released=True.

    released = [v for v in SaltVersionsInfo.versions() if v.released]
    assert released, "SaltVersionsInfo table has no released codenames"
    expected = released[-1]
    current = SaltVersionsInfo.current_release()
    assert current == expected, "..."

Same regression-protection value against the original #67061 bug
(current_release() walking to the first *un*-released codename would
still fail this assertion), zero per-release-cycle churn. Failure
message names both the expected and actual codenames.

Verified locally: contract-based assertion passes with the current
SaltVersionsInfo table state on this branch (expected=Argon/3008,
current=Argon/3008).
[3008.x] ci: skip CI on saltstack/salt-nightlies (duplicate of upstream)
…name

#70161 marked ARGON released=True on master so
SaltVersionsInfo.current_release() now returns ARGON. The regression
test test_current_release_matches_maintenance_branch_67061 was
asserting the pre-flip codename by name (CHLORINE), which needed
bumping to ARGON. Rather than bump the hardcoded value with every
release cycle (and reintroduce this same failure mode next time
POTASSIUM or its successors flip to released=True), assert the
*contract* that current_release() is documented to satisfy: return
the last codename in SaltVersionsInfo.versions() with released=True.

    released = [v for v in SaltVersionsInfo.versions() if v.released]
    assert released, "SaltVersionsInfo table has no released codenames"
    expected = released[-1]
    current = SaltVersionsInfo.current_release()
    assert current == expected, "..."

Same regression-protection value against the original #67061 bug
(current_release() walking to the first *un*-released codename would
still fail this assertion), zero per-release-cycle churn. Failure
message names both the expected and actual codenames.

Sibling PR: 3008.x -- SaltProject/salt#70167.
…on-3008.x

[3008.x] test_version: assert current_release() contract, not a hardcoded codename
…on-master

test_version: assert current_release() contract, not a hardcoded codename
Commit f3ffc8f (originally landed on 3007.x) added
`--match v3007.*` to salt.version's git-describe call to keep 3008.x
tags from hijacking version detection on 3007.x when a reverted
mis-merge left 3008.x commits reachable.

That commit got merged forward into 3008.x on 2026-08-25 (via
17c1f44 "Merge remote-tracking branch 'origin/3007.x' into
merge/3007.x/3008.x-08-25-26") without updating the constraint to
this branch's major. Result on 3008.x: git describe skips every
v3008.* tag and reports "v3007.14-N-gSHA". Downstream:

- __discover_version() sees parsed.major=3007 < saltstack_version.major=3008
  and takes the "lift baseline" branch, replacing the parsed base
  with SaltVersionsInfo.current_release() (=ARGON, info=3008 only,
  no minor).
- Minor defaults to 0.
- Every nightly RPM/DEB on 3008.x since the merge has been labeled
  salt-3008.0+N-... instead of salt-3008.2+N-...

Fix is one-line: change v3007.* -> v3008.*. Same intent as the
original 3007.x commit (constrain to the branch's own major),
adapted to 3008.x's calver line. git describe now returns
v3008.2-<ahead>-gSHA, __discover_version() parses it directly
(same major, no lift-baseline needed), and RPM/DEB packages get
the correct salt-3008.2+N-... version prefix.

Verified in the salt-nightlies 3008.x run 33122447777 (post-#70162
merged): produces salt-3008.0+2651.gfd8241adb4 instead of the
expected salt-3008.2+2651.gfd8241adb4. This one-line fix restores
the correct minor.

Master is unaffected -- it uses `--match v[0-9]*` (no branch-major
constraint).
…on-3008.x

[3008.x] version: constrain git-describe --match to v3008.* on 3008.x
Aligns the salt-nightlies visibility page
(https://saltstack.github.io/salt-nightlies) with the visual language
used across saltproject.io, docs.saltproject.io, and the install guide
(all pydata-sphinx-theme or its Hugo port).

Changes to .github/scripts/generate_nightly_dashboard.py:

- Embed SaltProject_altlogo_teal.png as a base64 data URI, sourced from
  the salt-install-guide _static/img/ set (identical to docs sites).
- Add a top navbar with logo + links to saltproject.io / install-guide
  / docs / GitHub, with the current page marked active.
- Swap the CSS palette to PST tokens: primary teal #0a7d91,
  link-hover purple #8045e5, success #28a745, danger #d72d47,
  warning #9a6700, background #fff, borders #d1d5da.
- Switch fonts to the PST system-UI + ui-monospace stacks.
- Add a footer with the "Updated ..." timestamp and repo link.

Zero external dependencies: no CDN CSS/JS, everything (logo + styles)
is self-contained in the generated index.html so the page renders on
enterprise GHE / air-gapped mirrors identically.

Rendered locally against the live gh-pages history.json — no regression
in table structure and the expand-on-click detail rows still work.
dashboard: adopt PyData Sphinx Theme palette + logo, matching docs
…-08-28-26

# Conflicts:
#	.github/workflows/ci.yml
#	.github/workflows/dependabot-sync.yml
#	.github/workflows/nightly-stress-test.yml
#	.github/workflows/nightly.yml
#	.github/workflows/scheduled.yml
#	.github/workflows/staging.yml
#	.github/workflows/templates/layout.yml.jinja
#	.pre-commit-config.yaml
#	pkg/macos/install_salt.sh
#	requirements/base.txt
#	tests/pytests/pkg/integration/test_version.py
#	tests/pytests/unit/cli/test_batch.py
#	tests/pytests/unit/grains/test_core.py
end-of-file-fixer flagged two of these scratch files for missing
final newlines. They are transient outputs from the silent-drop
audit sub-agent and shouldn't be tracked in the merge PR. The
audit report itself (agents/reports/silent_drop_audit_3008_to_master.md)
stays.
schedule/workflow_dispatch triggers only ever run a workflow as it
exists on the default branch. 3008.x and 3006.x each already carry
their own copy of this file with the same schedule block, but it has
never fired for either (confirmed via the Actions run history: every
event:schedule run is on master).

Matrix the existing job over branch: [master, 3008.x, 3006.x] with an
explicit `ref: matrix.branch` checkout, so master's live schedule
drives all three -- each running its own checked-out tests/monitoring/
code. Branch-qualify the Docker layer cache key, the uploaded artifact
name, and the stress-snapshots publish directory, since matrixed jobs
share one github.run_id/github.sha and would otherwise collide.

3007.x is left out: its tests/monitoring/ predates render_panels.py,
so the Render Dashboard Panels / Publish panels / Panel Summary steps
would fail against it as-is.
Scoped to this branch only -- lets a manual workflow_dispatch actually
execute (to verify the checkout-against-fork failure from earlier is
fixed by testing directly against this repo's own 3006.x/3008.x) without
touching the repo-wide variable, which would also affect master's real
schedule and anyone else relying on it. Must be reverted before merge.
VCOPS-100029: today there's no way to run this stress test against a
specific PR/branch's diff -- workflow_dispatch only exposes `duration`,
and the matrix's checkout ref was hardcoded to [master, 3008.x, 3006.x]
regardless of which ref you dispatched against.

Compute the matrix branch list from the trigger type instead of a fixed
array: schedule (nightly cron) still expands to the same three-branch
set unconditionally; workflow_dispatch expands to a single-element
array built from the new `branch` input (default "master", but any
branch/PR ref the caller types in). event_name=='schedule' short-
circuits before `inputs.branch` is evaluated, since schedule events
have no `inputs` context.
Drops the matrix/fromJSON trick entirely. nightly-stress-test.yml goes
back to a single job with one branch input (default master) -- whoever
dispatches it picks exactly one branch, full stop, no schedule trigger
of its own anymore.

Nightly coverage of master/3008.x/3006.x now comes from a new sibling
workflow, nightly-stress-test-dispatch.yml, which holds the only live
schedule (registered on master, per GitHub's default-branch-only rule)
and fires nightly-stress-test.yml three separate times via
workflow_dispatch -- once per branch, each its own independent run
rather than a matrix leg sharing one. Always dispatches against
--ref master so every run loads the same simple job definition,
threading the actual branch through as an input instead.

Since each dispatch is now its own run, matrix-driven collisions
(shared github.run_id/github.sha across legs) no longer apply --
dropped the RUN_DIR/artifact-name branch-qualification comments that
were about avoiding those, kept the qualification itself for
readability. Still need inputs.branch (not github.sha alone) in the
Docker cache key: every dispatch shares master's github.sha (since
dispatch ref is always master), but checks out different code per
inputs.branch, so a sha-only key would serve one branch's cached
layers to another.

3007.x still deliberately excluded from the nightly set (predates
render_panels.py).
The dispatcher (schedule-triggered, fires the nightly set) now owns the
nightly-stress-test.yml name -- that's the one people actually mean by
"the nightly stress test". The single-run building block (branch +
duration inputs, one job) moves to run-stress-test.yml, invoked both by
the dispatcher and by anyone manually testing a specific branch/PR.

Also trimmed the `branch` input description down to just "Branch", and
dropped a comment on the stress-snapshots RUN_DIR that was explaining
something not worth explaining.
It gates whether the *nightly* firing happens, not whether a stress
test can run at all -- it belongs on nightly-stress-test.yml's dispatch
job, not on run-stress-test.yml's, so that manually running
run-stress-test.yml against a PR branch (VCOPS-100029) still works
regardless of this repo-level variable.

Also drops the TEMPORARY if:true bypass from run-stress-test.yml now
that the real check has moved off of it entirely.
Mirrors run-nightly.yml's trigger-branch-nightly-builds job: use
--ref matrix.branch (via strategy.matrix) instead of always
--ref master, so each branch's own copy of run-stress-test.yml governs
its run instead of borrowing master's. This means 3008.x and 3006.x
each need their own run-stress-test.yml landed (separate PRs against
those base branches, coming next) -- master alone isn't enough anymore.
Per Option A (single source of truth on master): rather than 3008.x
keeping its own divergent copy of this workflow just to expose these
two inputs, fold them into master's copy as universal, opt-in inputs
available to every branch dispatched against.

worker_threads defaults to empty (not 3008.x's original '5') --
master's and 3006.x's tests/monitoring/master.conf both currently ship
worker_threads: 10, and a hardcoded '5' default would silently nudge
their runs into an unrequested config change + restart the first time
someone runs this without setting the input. Only a non-empty explicit
value touches master.conf now.
Daniel Wozniak (core maintainer) confirmed: workflow_dispatch --ref X
runs the copy of the target workflow file that exists ON BRANCH X, not
master's copy checked out against X. The previous design (--ref master
for every dispatch, reusing master's single-branch-input copy of
run-stress-test.yml, with inputs.branch only steering the actions/checkout
step inside it) therefore never actually picked up a change made to
run-stress-test.yml on a non-master branch -- editing 3006.x's copy of
that workflow would silently have zero effect on 3006.x's nightly run.

Now each branch in the loop dispatches with --ref matching itself, so
that branch's own copy of run-stress-test.yml governs its own nightly
run.

This file is meant to be byte-identical across master/3006.x/3007.x/
3008.x once each has it -- porting to a not-yet-covered branch is a
straight copy of the whole file, not a re-edit of the branch list.
Only master's copy is ever actually fired by the `schedule:` cron
(GitHub only honors schedule off the default branch); the branch list
still lives here (not only reasoned about on master) so a manual
workflow_dispatch of this file on another branch behaves the same as
the real nightly run would, and so porting requires no re-derivation.

3008.x is in the branch list already even though it doesn't have its
own run-stress-test.yml yet -- that dispatch will fail until it's
ported (companion PR), and `|| echo ...` keeps that one failure from
blocking master/3006.x in the same run. 3007.x stays out of the list
entirely (not commented out) -- its tests/monitoring/ predates
render_panels.py, so the Render Dashboard Panels / Publish panels /
Panel Summary steps would fail against it even once ported.

Also fixes a stale comment on run-stress-test.yml's Docker cache key
that described the old always-master dispatch behavior.
The dispatcher passed -f duration=30m, hardcoding a value that
duplicates (and could silently drift from) run-stress-test.yml's own
default of 0.5h. enable_metrics/worker_threads were never passed here
either. Drop the redundant duration override so the dispatcher only
sets what it actually needs (branch), leaving every other input to the
called workflow's own defaults.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t in run-nightly-stress.yml

nightly-stress-test.yml looping over master/3006.x/3008.x internally
meant any explicit workflow_dispatch of it (e.g. run-nightly-stress.yml
waking up 3006.x's otherwise-inert schedule) re-triggered all three
branches again, double-dispatching run-stress-test.yml for branches
already covered elsewhere. Match nightly.yml/run-nightly.yml's existing
split instead: nightly-stress-test.yml drops its own schedule and only
ever dispatches run-stress-test.yml for the branch it's running on
(github.ref_name); run-nightly-stress.yml's matrix becomes the sole
cron entry point and the sole branch list, now including master.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Match nightly.yml's shape exactly: one file that IS the worker,
triggered only by workflow_dispatch, no separate forwarder-plus-worker
pair. run-nightly-stress.yml already calls nightly-stress-test.yml
directly via --ref with no -f flags (mirroring run-nightly.yml calling
nightly.yml the same way), so the extra run-stress-test.yml hop was
unnecessary indirection. github.ref_name replaces the removed
inputs.branch throughout (cache key, artifact name, snapshot run path).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
run-nightly-stress.yml's trigger job already gates on this var before
ever dispatching nightly-stress-test.yml, so checking it again here
was redundant for the automated path -- and actively wrong for a
manual workflow_dispatch, since the var's whole purpose (per its own
comment) is capping automated nightly cost, not blocking a human who
deliberately ran this. nightly.yml (the equivalent worker for regular
nightly builds) carries no such check either; only its dispatcher
(run-nightly.yml) does.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This PR's scope shrinks to just run-nightly-stress.yml's matrix
change. nightly-stress-test.yml's rewrite (dispatcher-loop removal,
schedule removal, SKIP_NIGHTLY_STRESS_TEST dedup) is being established
on 3006.x first (#70197) and will be ported to master and 3008.x as
separate, later changes -- not bundled into this PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants